-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for QOA (Quite OK Audio) format #88646
Conversation
f3e71e5
to
376d6e6
Compare
I know image files in Godot have their importer somehow enforced to be Texture2D at first. Maybe the first issue could be fixed like this? |
I believe it has something to do with |
get_import_order is used to determine the import order of resource types and has no effect in the order of being chosen. For example, if an importer for a Regardless, this helped me find the actual function we need to override, which is get_priority, which actually affects order of being chosen. I have overriden that to return 0.9 so ResourceImporterWAV gets a higher priority. This is not how image importing does it but works anyway. With this fixed, I think the only thing left are some reviews. |
7564429
to
edc3156
Compare
Is it possible for the priorities to be adjusted so they're integers (in a float)? Thoughts? |
</member> | ||
<member name="data" type="PackedByteArray" setter="set_data" getter="get_data" default="PackedByteArray()"> | ||
Contains the audio data in bytes. | ||
You can load a file without having to import it beforehand using the code snippet below. Keep in mind that this snippet loads the whole file into memory and may not be ideal for huge files (hundreds of megabytes or more). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can load a file without having to import it beforehand using the code snippet below. Keep in mind that this snippet loads the whole file into memory and may not be ideal for huge files (hundreds of megabytes or more). | |
You can load a file without having to import it beforehand using the code snippet below: |
The "Keep in mind [...]" thing could go below the codeblocks.
Also I feel like putting this example inside data
instead of the leading description of the class is somewhat weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is copypasted from AudioStreamMP3.xml. If we should modify it, the changes should go there first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no suggestions documentation-wise, then. It's all copy-pasted from the others. It... definitely needs to be cleaned up at some point.
There's also some codestyle oddities in the QOA implementation itself, but I am to assume it's also all copypasted from the source and I would not touch them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not everything is copypasted. I would like to know what are the oddities.
Seems like For this to work properly we would have to make ResourceImporterWAV's return 2. Should it be done this way? |
f464445
to
5ff42a9
Compare
Wave cannot be deprecated since it is the majority of audio inputs from libraries I don’t recommend removing the music code from QOA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, it works as expected.
In terms of quality, I can't spot the difference with the uncompressed WAV source on my Audeze Maxwell headphones. There's no doubt this sounds much better than IDA-ADPCM.
With a 38-second 44,100 Hz, 16-bit stereo audio file, the uncompressed WAV is 6.5 MB and the corresponding imported QOA file (in .godot/imported
) is 1.3 MB. This is a reduction of over 5 MB, which is much greater than the file size added to export templates by this PR. Even if you only had a few seconds of mono audio to compress to QOA, you'd still easily save the ~25 KB added by this PR.
My only concern is with the different import mode required, as we don't use this approach for any other audio format (unless the source format is different, like Ogg Vorbis and MP3). I don't have a good answer to this though, as refactoring it sounds difficult.
There's also the alternative of decoding Ogg Vorbis/MP3 to uncompressed audio at load-time which could work well, at the cost of requiring more memory (it would have to be something you enable on import on specific sound effects).
Testing project: test_pr_88646.zip1
Footnotes
-
Music in the testing project by Marc A. Pullen. ↩
b6881a0
to
f0d3945
Compare
Something I was writing before reviews I added a greatly optimized version of the reference player's logic inside AudioStreamPlaybackQOA. This makes the diff smaller and reduces a lot of unnecessary callbacks. The patches in qoa.h are now solely meant to remove unused functions and fix errors/warnings when including it instead of things like "add reference player". |
Technically, this is an importer for QOA files. It has just been enabled to import from WAV files too, with more parameters, because at the present moment QOA converters are uncommon (currently, FFmpeg master supports QOA decoding but not encoding).
As far as I understood, one of the motivations behind QOA's creation was to avoid having to do this. Considering QOA is already cheap to decode, I believe it's better than sacrificing even a few KBs of memory. |
Tested a web release template build (platform=web target=template_release production=yes, enabling and disabling qoa). The size penalty of the resulting zip was only 4.7KiB. |
ff60127
to
568300d
Compare
bc85979
to
166115b
Compare
Ping-Pong and Backward loops successfully implemented.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last remaining criticism I had of the pull regarding missing Ping-Pong and Backward loops was solved, so I don't have any arguments against merging QOA. Maybe @reduz or the audio team can look, but we don't have an active audio team.
Approving the enhancement, didn't review the technical implementation deeply.
I have changed the stream to use a Feature-wise, the only thing missing is |
- Remove loop count, it's not present on WAV; - Rename sign to increment; - Remove channels in favor of stereo to match WAV;
Superseded by #91014 |
Superseded by #91014. |
Implements support for the Quite OK Audio format (QOA). Closes godotengine/godot-proposals#9133. More details on that proposal.
QOA was developed to (not exactly just this) be a better alternative to IMA-ADPCM for use in games according to the article announcing it (which unironically uses Godot as an example).
Briefly, this adds a way for developers to compress sound effects without relying on expensive decoding (MP3, Ogg Vorbis) or destroyed quality (IMA-ADPCM).
A deprecation notice could be added later for when someone tries to import WAV as IMA-ADPCM (getting godotengine/godot-proposals#4264, where QOA was first mentioned, closed too). Adding a converter from WAV was suggested there.
The final release template binaries (at least on Linux) have a size increase of only 24KiB, but it gets compensated if at least 0.18-0.35s of WAV sound gets converted to QOA.
qoa.h
had to be patched to suppress a few warnings.Approaches
1. QOA files can be imported directly
2. WAV files can be imported to use QOA internally
Caveats
Issues to address in case this is accepted:
WAVs get imported as Quite OK Audio instead of Microsoft WAV by default;get_priority
to return 0.9 instead of the default value of 1.0;